fix: Use org unit stored path [DHIS2-18234] #19626
Conversation
larshelge
changed the title
larshelge
changed the title
|
Yes agree @jbee , I noticed that if you make a change using the maintenance app > org unit hierarchy operations by moving e.g. an org unit at level 3 to under another org unit at level 2, the |
|
I also found that and then got reminded that this is "by design" or at least a known issue because we do have a dedicated action in the maintenance controller to then run a full update. Jason P. told me that this is something implementations use regularly when they suspect something is off.
@larshelge We had a long chat about the issue in November, I think, and the TLDR is: it is really complicated problem to make this consistent. Most likely our best bet is to utilize some sort of DB functionality to update paths based on parent reference and not have any logic in Java. But for all the "simple" ideas we quickly found reasons why it does not work but only shifts the problem to somewhere else. |
|
My understanding of this PR is that it might help with the issue concerned, but is yet to be proven. Should we not try to prove there is some improvement before merging to master? Liaise with the reporter and ask them to try the proposed fix? |
|
I mostly agree @david-mackessy , I am working on a "backport" to 40, after which I will create a custom build and ask for feedback. That said, I see this as a general improvement which I suggest should be merged whether it fixes DHIS2-18234 or not. Since the |
|
@jbee agree on your comments on moving complexity around. Perhaps i) running the path update function after a an explicit hierarchy operation is triggered, and ii) having a system job which updates the paths nightly will be a good enough solution. |
Introduces a new method
getStoredPath()onOrganisationUnit.Note that the
pathproperty is mapped with the "property access" mode in Hibernate. Per the Hibernate docs:This means that Hibernate will call
getPath()and set thepathproperty as part of all persistence operations. This in turn means that we don't have to recalculate the path value in the DHIS 2 application every time we need the value, as it will be persisted in the database.The exception is when code modifies the ancestors/parents and relies on an up-to-date path value. Though in those special cases, one can simply call the
getPath()method. In all other scenarios, thegetStoredPath()method can be called, which should introduce a significant performance gain.For the
getStoredPath()method, if the path is not defined, typically as part of an integration test where the state is not yet flushed to the database, the calculated path based on the org unit ancestors is returned.Reducing the pressure from re-calculating the path may alleviate the problem of DHIS2-18234 where the UID of the ancestor org units are not reflecting the persisted value.